Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables the prerelease_deps and mypy sessions in noxfile.py, which were previously skipped or commented out. The prerelease_deps session now runs unit and system tests, and the mypy session is fully configured with its own dependencies and execution steps. Feedback includes a correction to the --exclude flag in the mypy command to ensure the regex is interpreted correctly by removing internal quotes, as well as a suggestion to use a version constant instead of a hardcoded Python version for better maintainability.
| os.path.join("tests", "unit"), | ||
| "--check-untyped-defs", | ||
| "--explicit-package-bases", | ||
| '--exclude="^third_party"', |
There was a problem hiding this comment.
The --exclude flag contains literal double quotes within the string ("^third_party"). Since session.run passes arguments directly to the executable without shell interpretation, these quotes will be treated as part of the regular expression, likely causing it to fail to match the intended third_party directory. It should be passed without the internal quotes.
| '--exclude="^third_party"', | |
| "--exclude=^third_party", |
packages/bigframes/noxfile.py
Outdated
|
|
||
| @nox.session(python=DEFAULT_PYTHON_VERSION) | ||
| # NOTE: this is the mypy session that came directly from the bigframes split repo | ||
| @nox.session(python="3.10") |
There was a problem hiding this comment.
Hardcoding python="3.10" for the mypy session is less flexible than using a version constant like DEFAULT_PYTHON_VERSION or ALL_PYTHON[-1], which are used elsewhere in this file. Unless there is a specific requirement to run type checks only on Python 3.10, consider using the project's default Python version for consistency and easier maintenance.
| @nox.session(python="3.10") | |
| @nox.session(python=DEFAULT_PYTHON_VERSION) |
…ke prerelease.cfg
1 participant
This PR seeks to enable a number of tests to help ensure full validation.
See #16489 for additional details.